-
-
Notifications
You must be signed in to change notification settings - Fork 926
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Recipes draft #1003
Recipes draft #1003
Conversation
Thank you!Thank you for your pull request 😃 🤖 This automated message can help you check the rendered files in your submission for clarity. If you have any questions, please feel free to open an issue in {sandpaper}. If you have files that automatically render output (e.g. R Markdown), then you should check for the following:
Rendered Changes🔍 Inspect the changes: https://github.com/swcarpentry/git-novice/compare/md-outputs..md-outputs-PR-1003 The following changes were observed in the rendered markdown documents:
What does this mean?If you have source files that require output and figures to be generated (e.g. R Markdown), then it is important to make sure the generated figures and output are reproducible. This output provides a way for you to inspect the output in a diff-friendly manner so that it's easy to see the changes that occur due to new software versions or randomisation. ⏱️ Updated at 2024-09-18 12:43:15 +0000 |
@nsoranzo thank you so much for your suggestions. After searching around for established conventions, I agree both with the idea of capitalizing Git and with using present tense rather than present continuous in commit messages. However, these are changes that are unrelated to this pull request. Any reason why you wanted to suggest this here? It would be best to create another PR after this one has been merged (if it's approved). Thank you though. |
Thanks for looking into my suggestions! I partially agree with your reply:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the whole, this looks good and results in less of a change (in practical terms) than I had expected. I have suggested a minor change to the Markdown syntax used in the examples: having multiple h1 headings in a file is bad practice from an accessibility standpoint, so I have suggested h2 headings (##
) instead.
A few other miscellaneous questions and suggestions too.
Co-authored-by: Toby Hodges <tobyhodges@carpentries.org>
Markdown accepts either `-` or `*` however, in this material, the `-` causes confussion when looking at diffs as you'd get things like: ```diff - avocado -- lemon +- lime ``` making the `-` from the diff misunderstood with the `-` of the list.
This is done to have files with a single heading as accessibility recommendations. A title is introduced (recipe title) and the ingredients and instructions are set as 2nd level headings.
If and when #1013 is merged, I will be happy to approve this. |
Ups! I didn't see your comment, @tobyhodges - I merged also #1012 into #1013 because it was producing a lot of merge conflicts. |
Co-authored-by: Nicola Soranzo <nicola.soranzo@earlham.ac.uk>
Co-authored-by: Nicola Soranzo <nicola.soranzo@earlham.ac.uk>
Co-authored-by: Toby Hodges <tobyhodges@carpentries.org>
Co-authored-by: Toby Hodges <tobyhodges@carpentries.org>
I've incorporated the suggestions by @dpshelio, @nsoranzo, and @tobyhodges. The only conversation left is about the sentence "A supplemental episode to this lesson discusses advanced setup and concepts of SSH and key pairs, and other material supplemental to git related SSH" that was introduced, and probably refers to something internal to UCL, I'm guessing. I will now proceed and merge #1013. After that, we can all set a date for merging. At that point, I will rebase and fix the conflicts. |
Modifies examples to include a single heading and changes md symbol used for listing items
Nothing to do with UCL 😇 - that was introduced in 2021 by @kekoziar |
Hello! I'm a git-novice-es-maintainer
|
This is so cool! Thank you for the the revisions and work! |
These look like great improvements, many thanks to all who worked on them! |
Cody,
It looks fairly complete and would be ready for teaching next week. Just a
bit tricky to share the right website? ~~ jaws
…On Fri, Sep 13, 2024, 10:40 AM Cody Hennesy ***@***.***> wrote:
These look like great improvements, many thanks to all who worked on them!
@kekoziar <https://github.com/kekoziar> @martinosorb
<https://github.com/martinosorb>: is there a target date for approving
the PR? I have instructors planning to teach this on Sept 26, and want to
make sure we know which version will be live since these are pretty
significant changes.
—
Reply to this email directly, view it on GitHub
<#1003 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMT5MX5F6GDUE7GU4XDBGJLZWMBPPAVCNFSM6AAAAABKKEZRESVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBZGI2DQMRRG4>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Paging @tobyhodges on timing. If you guys would like to have this version for your workshop, I think we could arrange to have it merged before then. |
Thanks @martinosorb @tobyhodges Our instructors noted they won't have much time to review the changes in their prep before their workshop so are wondering if it would be OK to wait until after Sept 26, 2024 1800 UTC to merge the PR? |
Co-authored-by: James Foster <38274066+jd-foster@users.noreply.github.com>
Thanks for catching that typo @jd-foster. |
Auto-generated via {sandpaper} Source : d4dfd52 Branch : md-outputs Author : GitHub Actions <actions@github.com> Time : 2024-09-29 16:46:10 +0000 Message : markdown source builds Auto-generated via {sandpaper} Source : ecbc85e Branch : main Author : Martino Sorbaro <martinosorb@users.noreply.github.com> Time : 2024-09-29 16:45:16 +0000 Message : Merge pull request #1003 from swcarpentry/recipes-draft Recipes draft
🎉 🎆 🥳 👏 👏 👏 👏 |
I created this PR to check if it was possible to merge the Recipes version without too much pain, and so we can discuss whether this is a good choice.
Before merging we have a number of TODOs: